Skip to content

CLOUDSTACK-10236: Enable dynamic roles for missing props file#2426

Merged
yadvr merged 2 commits intoapache:4.11from
shapeblue:dynamic-roles-upgrade-fix
Jan 24, 2018
Merged

CLOUDSTACK-10236: Enable dynamic roles for missing props file#2426
yadvr merged 2 commits intoapache:4.11from
shapeblue:dynamic-roles-upgrade-fix

Conversation

@yadvr
Copy link
Member

@yadvr yadvr commented Jan 23, 2018 •
edited
Loading

  • In case commands.properties file is missing, enables dynamic roles.
  • Adds a new -D or --default flag to migrate-dynamicroles.py script
    to simply update the global setting and use the default role-rule
    permissions, without needing rules from old command.properties file.

@blueorangutan package

Pinging for review - @borisstoyanov @DaanHoogland @nvazquez @wido @rafaelweingartner @PaulAngus and others.

Example, dynamic roles migration script run:

> python scripts/util/migrate-dynamicroles.py -d --default
Apache CloudStack Role Permission Migration Tool
(c) Apache CloudStack Authors and the ASF, under the Apache License, Version 2.0

Applying the default role permissions, ignoring any provided properties files(s).
Running SQL query: UPDATE `cloud`.`configuration` SET value='true' where name='dynamic.apichecker.enabled'
Dynamic role based API checker has been enabled!

@yadvr
CLOUDSTACK-10236: Automate dynamic roles migration for missing props …
f19a110 
…file

- In case commands.properties file is missing, enables dynamic roles.
- Adds a new -D or --default flag to migrate-dynamicroles.py script
  to simply update the global setting and use the default role-rule
  permissions.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr yadvr added this to the 4.11.0 milestone Jan 23, 2018
@blueorangutan
Copy link

blueorangutan commented Jan 23, 2018

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Jan 23, 2018

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1673

@yadvr
Copy link
Member Author

yadvr commented Jan 23, 2018

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Jan 23, 2018

@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

blueorangutan commented Jan 24, 2018

Trillian test result (tid-2185)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39834 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2426-t2185-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_public_ip_range.py
Intermitten failure detected: /marvin/tests/smoke/test_templates.py
Intermitten failure detected: /marvin/tests/smoke/test_usage.py
Intermitten failure detected: /marvin/tests/smoke/test_volumes.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 63 look OK, 4 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_04_extract_template Failure 128.36 test_templates.py
ContextSuite context=TestISOUsage>:setup Error 0.00 test_usage.py
test_06_download_detached_volume Failure 138.70 test_volumes.py
test_04_rvpc_network_garbage_collector_nics Failure 291.01 test_vpc_redundant.py

borisstoyanov
borisstoyanov approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT, the errors reported by marvin are not related to these changes

@yadvr
Copy link
Member Author

yadvr commented Jan 24, 2018

@borisstoyanov can you help with upgrade related testing?

@borisstoyanov
Copy link
Contributor

borisstoyanov commented Jan 24, 2018

Yes, I'm on it @rhtyd

DaanHoogland
DaanHoogland approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@yadvr
Copy link
Member Author

yadvr commented Jan 24, 2018 •
edited
Loading

We've enough code review and test results LGTM. I'll wait for @borisstoyanov 's test results, then we may merge.

@yadvr
Add warning message, ask admins to move to dynamic roles
273f733 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
DaanHoogland
DaanHoogland approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no comments

borisstoyanov
borisstoyanov approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LMGT

@yadvr
Copy link
Member Author

yadvr commented Jan 24, 2018

Thanks, merging this based on code reviews and test results.

@yadvr yadvr merged commit 170b6ce into apache:4.11 Jan 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@borisstoyanov borisstoyanov borisstoyanov approved these changes

Assignees

No one assigned

Labels

component:upgrade Severity:BLOCKER

Projects

None yet

Milestone

4.11.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@yadvr @blueorangutan @borisstoyanov @DaanHoogland